-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add specific driver for Vimar Touch Thermostats of type 02952.b #29
base: master
Are you sure you want to change the base?
Conversation
That can set the shift to 0 and thus resetting the setpoint to the base of current mode
Excuse the delay, We are discussing this internally |
No problem, no rush for me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget to translate the setting labels/hints and the flow labels to dutch if you are able to.
Also it would be nice to have a custom icon and images for this device. You can request the icon here: https://github.com/athombv/homey-vectors-public
this.registerCapabilityListener('vimar_thermostat_mode', this.onCapabilityMode.bind(this)); | ||
|
||
// Maybe this can be placed better during pairing? | ||
if (!this.settings.ga_summerwinter_state && this.hasCapability('summer_winter')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a bug here. If you dont fill in all the adresses during pairing you still see these capabilities
super.onInit(); | ||
|
||
this.homey.flow.getActionCard('set-window-switch').registerRunListener(async (args, state) => { | ||
return args.device.knxInterface.writeKNXGroupAddress(args.device.settings.ga_window_switch, args.open, 'DPT1') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check if group adress is available
}); | ||
}); | ||
|
||
this.homey.flow.getActionCard('reset_to_basesetpoint').registerRunListener(async (args, state) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would someone want to set the target temperature to 0?
</div> | ||
</div> | ||
</fieldset> | ||
<fieldset id="vimar-thermostat-02952-b-fieldset" style="display: none"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you might want to mention that some adresses are optional
"getable": true, | ||
"setable": false, | ||
"uiComponent": "sensor" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sensors probably need a capability icon as well, otherwise they will appear with an empty placeholder in the UI.
And before going into details on this implementation, I think it is worth to think about a future where all devices might not share the .homeycompose/drivers/pair/select_groupaddresses/index.html
Maybe there is a way to make it dynamic based on the settings object?
From a feature point of view this device is functioning quite well (running it now) but I am unsure about how you see the future of specific devices.
I specifically made this because it is a device that absolutely does not fit into the existing thermostat driver, and it is so specific that I don't think it would make sense to create a generic setpoint shift type thermostat.
What do you think? Because I don't see a way (new to Homey) that we could make a system like Zigbee/BLE/ZWave where other apps could be developed for specific devices? They all have to be in this repository, right?